-
Notifications
You must be signed in to change notification settings - Fork 117
test(l2): fetch fee vault addresses from the RPC #5183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Lines of code reportTotal lines added: Detailed view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for retrieving the L1 fee vault address dynamically from the L2 RPC server instead of relying on environment variables. The change enables the system to handle optional fee vaults (returning None when not configured) and removes the need for the INTEGRATION_TEST_SKIP_BASE_FEE_VAULT_CHECK environment variable.
- Added new RPC method
ethrex_getL1FeeVaultAddressto retrieve L1 fee vault address - Changed fee vault functions to return
Option<Address>instead ofAddressto support optional vaults - Refactored test code to fetch vault addresses dynamically and skip assertions when vaults are not configured
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/networking/rpc/clients/eth/errors.rs | Added error types for the new L1 fee vault address RPC method |
| crates/l2/networking/rpc/rpc.rs | Registered the new ethrex_getL1FeeVaultAddress RPC method handler |
| crates/l2/networking/rpc/l2/fees.rs | Implemented GetL1FeeVaultAddress handler following existing patterns for fee vault retrieval |
| crates/l2/networking/rpc/clients.rs | Added client function to call the new RPC method and updated return types to Option<Address> |
| crates/l2/tests/tests.rs | Refactored tests to dynamically fetch vault addresses and handle optional vaults, removed hardcoded addresses |
| .github/workflows/pr-main_l2.yaml | Removed INTEGRATION_TEST_SKIP_BASE_FEE_VAULT_CHECK environment variable from CI workflows |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Motivation
Currently, our integration tests use default
fee vaultaddresses, and to run the tests with custom ones, we need to pass them through env vars.Description
Closes None